Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

4.x - OpenAPI updates #7669

Merged
merged 3 commits into from
Sep 27, 2023
Merged

4.x - OpenAPI updates #7669

merged 3 commits into from
Sep 27, 2023

Conversation

romain-grecourt
Copy link
Contributor

@romain-grecourt romain-grecourt commented Sep 26, 2023 •
edited
Loading

  • Minimalistic SE OpenAPI support with SPI to implement the MicroProfile support and OpenAPI UI
  • Make openapi a multi-module with two sub-modules: openapi and openapi-ui
  • Add openapi/tests and move gh-5792 from tests/integration
    • Re-enabled test, make it strictly a test (not an app) and use version.lib.snakeyaml to override the SnakeYAML version
  • Created OpenApiFormat to formalize what was before OpenApiFeature.OpenAPIMediaType
  • Microprofile OpenAPI refactorings:
    • MPOpenAPIBuilder -> FilteredIndexViewsBuilder as a utility to create List<FilteredIndexView>
    • MpOpenApiManager implements OpenApiManager using SmallRye OpenAPI (what was before in MpOpenApiFeature
    • Prefix utility classes with OpenApi:
      • ParserHelper -> OpenApiParser
      • Serializer -> OpenApiSerializer
    • Renamed HelidonAnnotationScannerExtension to JsonpAnnotationScannerExtension to remove 'Helidon' from the class name
    • Renamed tests to use Test as a suffix instead of prefix
  • Updated examples/openapi to remove the in-memory model related features (i.e. reader, filter)
  • Renamed examples/microprofile/openapi-basic to examples/microprofile/openapi (to be symetrical with SE)
  • Updated tests to use new testing patterns (i.e. helidon-microprofile-testing-junit5 for MP and helidon-webserver-testing-junit5 for SE)
  • Generated config docs for openapi/openapi, openapi/openapi-ui, microprofile/openapi (Removed old files)

Fixes #7247 (SE OpenAPI static file support)
Fixes #7240 (Fix gh-5792 integration test)
Fixes #6130 (Port OpenAPI UI integration to 4.x)
Fixes #7643 (OpenAPI parsing fails to handle default in some cases)
Fixes #7668 (Routing path with optional sequence not supported)

Documentation impact

OpenAPI UI Maven coordinates changes:

Old:

<dependency>
    <groupId>io.helidon.integrations.openapi-ui</groupId>
    <artifactId>helidon-integrations-openapi-ui</artifactId>
</dependency>

New:

<dependency>
    <groupId>io.helidon.openapi</groupId>
    <artifactId>helidon-openapi-ui</artifactId>
</dependency>

The change is included in this pull request.

In-Memory model related features are no longer supported in SE

This means the effective configuration available in SE is restricted to what is listed in docs/config/io_helidon_openapi_OpenApiFeature.adoc:

openapi:
    enabled: true
    web-context: "/openapi"
    static-file: "META-INF/openapi.yml"
    cors:
       # See io.helidon.webserver.cors.CrossOriginConfig

@romain-grecourt romain-grecourt added this to the 4.0.0-RC1 milestone Sep 26, 2023
@romain-grecourt romain-grecourt self-assigned this Sep 26, 2023
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 26, 2023
This was linked to issues Sep 26, 2023
4.x SE/Nima static OpenAPI file support via the /openapi endpoint #7247
Closed
[4.x] OpenAPI parsing fails to handle default in some cases #7643
Closed
4.x routing path with optional sequence not supported #7668
Closed
[4.x] Port OpenAPI UI integration to 4.x #6130
Closed
4.x fix tests in tests/integration/gh-5792 #7240
Closed
@romain-grecourt
OpenAPI updates.
f7b0e33 
- Minimalistic SE OpenAPI support with SPI to implement the MicroProfile support and OpenAPI UI
- Make openapi a multi-module with two sub-modules: openapi and openapi-ui
- Add openapi/tests and move helidon-iogh-5792 from tests/integration
  - Re-enabled test,  make it strictly a test (not an app) and use version.lib.snakeyaml to override the SnakeYAML version
- Created OpenApiFormat to formalize what was before OpenApiFeature.OpenAPIMediaType
- Microprofile OpenAPI refactorings:
  - MPOpenAPIBuilder into FilteredIndexViewsBuilder as a utility to create List<FilteredIndexView>
  - MpOpenApiManager implements OpenApiManager using SmallRye OpenAPI (what was before in MpOpenApiFeature
  - Prefix utility classes with OpenApi:
    - ParserHelper -> OpenApiParser
    - Serializer -> OpenApiSerializer
  - Renamed HelidonAnnotationScannerExtension to JsonpAnnotationScannerExtension to remove 'Helidon' from the class name
  - Renamed tests to use Test as a suffix instead of prefix
- Updated examples/openapi to remove the in-memory model related features (i.e. reader, filter)
- Renamed examples/microprofile/openapi-basic to examples/microprofile/openapi (to be symetrical with SE)
- Updated tests to use new testing patterns (i.e. helidon-microprofile-testing-junit5 for MP and helidon-webserver-testing-junit5 for SE)
- Generated config docs for openapi/openapi, openapi/openapi-ui, microprofile/openapi (Removed old files)

Fixes helidon-io#7247 (SE OpenAPI static file support)
Fixes helidon-io#7240 (Fix helidon-iogh-5792 integration test)
Fixes helidon-io#6130 (Port OpenAPI UI integration to 4.x)
Fixes helidon-io#7643 (OpenAPI parsing fails to handle default in some cases)
Fixes helidon-io#7668 (Routing path with optional sequence not supported)
tjquinno
tjquinno requested changes Sep 26, 2023
View reviewed changes
Copy link
Member

@tjquinno tjquinno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of stylistic changes as well as functional ones, but I think I made my way through them all!

One specific change: Please update the openapi.yml file in the example as follows:

diff --git a/examples/openapi/src/main/resources/META-INF/openapi.yml b/examples/openapi/src/main/resources/META-INF/openapi.yml
index 8a7c77a20..617333bc3 100644
--- a/examples/openapi/src/main/resources/META-INF/openapi.yml
+++ b/examples/openapi/src/main/resources/META-INF/openapi.yml
@@ -49,12 +49,10 @@ paths:
             examples:
               greeting:
                 summary: Example greeting message to update
-                value: New greeting message
+                value: { "greeting": "Hola" }
       responses:
-        "200":
-          description: OK
-          content:
-            application/json: {}
+        "204":
+          description: Updated
   /greet/{name}:
     get:
       summary: Returns a personalized greeting

This way the static OpenAPI document conforms better to the behavior of the PUT which changes the greeting (the status is 204, not 200) and the example should be JSON not plain text so the UI works correctly without the user having to edit anything. These are not changes which this PR introduced, but we might as well fix them now.

My one lingering concern, which we discussed earlier and I know you considered, is that our wrapper around the UI is in my mind very much an integration which is why I added it under integrations originally. To bring it under helidon/openapi as a submodule, to me, suggests a level of commitment to and therefore support by Helidon of the SmallRye and Swagger code that make up the UI which I would rather we did not convey.

openapi/openapi-ui/src/main/java/module-info.java Outdated Show resolved Hide resolved
@romain-grecourt
Incorporate review feedback:
06a5782 
- Use FQN the provide statement in openapi/openapi-ui/src/main/java/module-info.java
- Update META-INF/openapi.yml
@romain-grecourt
Copy link
Contributor Author

Lots of stylistic changes as well as functional ones, but I think I made my way through them all!

It's true that I made stylistic changes. Because I made so many changes (renames, compiler warnings etc.) I decided to keep the stylistic changes. However, if you feel strongly about this, let me know what to revert and I will comply (Sir, Yes Sir!)

One specific change: Please update the openapi.yml file in the example as follows:

I have made the change.

My one lingering concern, which we discussed earlier and I know you considered, is that our wrapper around the UI is in my mind very much an integration which is why I added it under integrations originally. To bring it under helidon/openapi as a submodule, to me, suggests a level of commitment to and therefore support by Helidon of the SmallRye and Swagger code that make up the UI which I would rather we did not convey.

Let's raise this concern with the team and get a consensus on what to do (either rollback the GAV change or keep it this way).

@romain-grecourt romain-grecourt merged commit a84fd42 into helidon-io:main Sep 27, 2023
12 checks passed
@romain-grecourt romain-grecourt deleted the openapi-updates-4.x branch September 27, 2023 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjquinno tjquinno tjquinno approved these changes

@tomas-langer tomas-langer Awaiting requested review from tomas-langer

Assignees

@romain-grecourt romain-grecourt

Labels
4.x Version 4.x OCA Verified All contributors have signed the Oracle Contributor Agreement. open-api
Projects
None yet
Milestone
4.0.0-RC1
2 participants
@romain-grecourt @tjquinno